[CCIP-4171] integration-tests/smoke/ccip: add re-org tests #16056
Conversation
AER Report: CI Coreaer_workflow , commit , Detect Changes , Clean Go Tidy & Generate , Scheduled Run Frequency , Core Tests (go_core_tests) , Core Tests (go_core_tests_integration) , Core Tests (go_core_ccip_deployment_tests) , Core Tests (go_core_fuzz) , Core Tests (go_core_race_tests) , GolangCI Lint (.) , test-scripts , GolangCI Lint (integration-tests) , GolangCI Lint (deployment) , lint , SonarQube Scan 1. Possible nil pointer dereference: Golang Lint (integration-tests)Source of Error:Why: The error indicates that Suggested fix: Ensure that AER Report: Operator UI CI ran successfully ✅ |
makramkd
requested review from
winder,
asoliman92,
dimkouv,
mateusz-sekara,
0xAustinWang and
0xnogo
|
| } | ||
|
|
||
| func (l *DeployedLocalDevEnvironment) GetDevEnvConfig() devenv.EnvironmentConfig { | ||
| return *l.devEnvCfg |
| @@ -293,7 +293,22 @@ const ( | |||
| Base64OverrideEnvVarName = k8s_config.EnvBase64ConfigOverride | |||
| ) | |||
|
|
|||
| func GetConfig(configurationNames []string, product Product) (TestConfig, error) { | |||
| // GetConfig returns a TestConfig struct with the given configuration names | |||
| evm_gas_estimation_buffer = 1000 | ||
| evm_supports_eip1559 = true | ||
| evm_default_gas_limit = 6000000 | ||
| evm_finality_depth = 10 |
There was a problem hiding this comment.
nit : is the chainlink node config updated as part of this updated finality? if not you can create a similar func like this to update the finality in evmConfig.Chain from the finality mentioned in blockchain.EVMNetwork input param. It's not updated in the existing function because other product do not use finality.
| #! FinalityDepth on 2337 overridden to 10. | ||
| 2337 = """ | ||
| AutoCreateKey = true | ||
| FinalityDepth = 10 |
There was a problem hiding this comment.
if you only need this because of Finality Depth , please check my other comment
| bnAfterReorg, err := reorgSourceClient.BlockNumber() | ||
| require.NoError(t, err, "error getting block number after reorg") | ||
|
|
||
| l.Info().Int64("blockNumberAfterReorg", bnAfterReorg).Msg("block number after reorg") |
There was a problem hiding this comment.
how about a check to validate whether reorg happened? bnAfterReorg < minChainBlockNumberBeforeReorg
| l.Info().Msgf("sent CCIP message that will get re-orged, msg id: %x", reorgingMsgEvent.Message.Header.MessageId) | ||
|
|
||
| // Run reorg above finality depth | ||
| const reorgDepth = 60 |
There was a problem hiding this comment.
would be easier to update if you put something like these into var
ReorgDepthGreaterThanFinality and ReorgDepthLesserThanFinality
| require.Eventually(t, func() bool { | ||
| violatedResponses := make(map[string]struct{}) | ||
| for _, node := range nodeAPIs { | ||
| // skip bootstrap nodes, they won't have any logpoller filters | ||
| p2pKeys, err := node.MustReadP2PKeys() | ||
| require.NoError(t, err) | ||
|
|
||
| l.Debug().Msgf("got p2pKeys from node API: %+v", p2pKeys) | ||
|
|
||
| require.GreaterOrEqual(t, len(p2pKeys.Data), 1) | ||
| if !slices.Contains(nonBootstrapP2PIDs, p2pKeys.Data[0].Attributes.PeerID) { | ||
| l.Info().Msgf("skipping bootstrap node w/ p2p id %s", p2pKeys.Data[0].Attributes.PeerID) | ||
| continue | ||
| } | ||
|
|
||
| resp, _, err := node.Health() | ||
| require.NoError(t, err) | ||
| for _, d := range resp.Data { | ||
| if d.Attributes.Name == destLogPollerService && | ||
| d.Attributes.Output == "finality violated" && | ||
| d.Attributes.Status == "failing" { | ||
| violatedResponses[p2pKeys.Data[0].Attributes.PeerID] = struct{}{} | ||
| break | ||
| } | ||
| } | ||
|
|
||
| if _, ok := violatedResponses[p2pKeys.Data[0].Attributes.PeerID]; ok { | ||
| l.Info().Msgf("node %s reported finality violation", p2pKeys.Data[0].Attributes.PeerID) | ||
| } else { | ||
| l.Info().Msgf("node %s did not report finality violation, log poller response: %+v", |
There was a problem hiding this comment.
nit: a wrapper func for checking the finality violation is reported and not reported given the chain details
| noreorgSourceSelector := allChains[1] | ||
| noreorgSourceChain, ok := chainsel.ChainBySelector(noreorgSourceSelector) | ||
| require.True(t, ok) | ||
| reorgLogPollerService := fmt.Sprintf("EVM.%d.LogPoller", reorgSourceChain.EvmChainID) |
| }, 3*time.Minute, 5*time.Second, "not all the nodes report finality violation") | ||
| l.Info().Msg("All nodes reported finality violation") | ||
|
|
||
| // expect the commit to still go through on the non-reorged source chain. |
There was a problem hiding this comment.
are you validating that reorged message is not processed
| }, 3*time.Minute, 5*time.Second, "not all the nodes report finality violation") | ||
| l.Info().Msg("All nodes reported finality violation") | ||
|
|
||
| // expect the commit to still go through on the non-reorged source chain. |
There was a problem hiding this comment.
are you validating that reorged message is not processed
|
|
||
| // wait for log poller filters to get registered. | ||
| l.Info().Msg("waiting for log poller filters to get registered") | ||
| time.Sleep(15 * time.Second) |
There was a problem hiding this comment.
This is mostly not enough in in-memory test. Until we have proper solution, I would bump this to 30 sec.
| testhelpers.WithLogMessagesToIgnore([]testhelpers.LogMessageToIgnore{ | ||
| { | ||
| Msg: "Got very old block.", | ||
| Reason: "We are expecting a re-org beyond finality", | ||
| Level: zapcore.DPanicLevel, | ||
| }, | ||
| { | ||
| Msg: "Reorg greater than finality depth detected", | ||
| Reason: "We are expecting a re-org beyond finality", | ||
| Level: zapcore.DPanicLevel, | ||
| }, | ||
| { | ||
| Msg: "Failed to poll and save logs due to finality violation, retrying later", | ||
| Reason: "We are expecting a re-org beyond finality", | ||
| Level: zapcore.DPanicLevel, | ||
| }, | ||
| }), | ||
| ) |
There was a problem hiding this comment.
We can extract this out as it is similar in the other test as well.
| } | ||
|
|
||
| l.Info().Msgf("%d nodes reported finality violation", len(violatedResponses)) | ||
| return len(violatedResponses) == nonBootstrapCount |
There was a problem hiding this comment.
How consistently is this works? In v1.5 test, we had flakiness in detecting the violation within given time by all nodes. I have tried some trade offs like certain number of nodes detection and all but it was still flakey. I see your approach of moving forward with certain blocks and then applying the re-org. Is that helps to produce consistent results?
Add re-org tests that cover the following cases: